Add basic U-mode support with ecall-based syscalls #53
+71
−4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Allows architecture-specific syscall implementations to override the generic dispatcher at link time for privilege separation.
Implements syscall interface using the ecall instruction to trap into M-mode from user space. Follows RISC-V calling convention for system call arguments and return values. Introduces a dedicated file for kernel entry mechanisms following Linux conventions. This separation clarifies the distinction between trap-based entry and generic syscall dispatch logic.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="arch/riscv/hal.c">
<violation number="1" location="arch/riscv/hal.c:334">
Syscall handler reads the wrong registers from the ISR frame, so the kernel receives an incorrect syscall number and arguments.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
arch/riscv/hal.c
Outdated
| isr_frame[31] = new_epc; | ||
|
|
||
| /* Extract syscall arguments from ISR frame */ | ||
| int syscall_num = isr_frame[17]; |
There was a problem hiding this comment.
Syscall handler reads the wrong registers from the ISR frame, so the kernel receives an incorrect syscall number and arguments.
Prompt for AI agents
Address the following comment on arch/riscv/hal.c at line 334:
<comment>Syscall handler reads the wrong registers from the ISR frame, so the kernel receives an incorrect syscall number and arguments.</comment>
<file context>
@@ -321,6 +321,31 @@ uint32_t do_trap(uint32_t cause, uint32_t epc, uint32_t isr_sp)
+ isr_frame[31] = new_epc;
+
+ /* Extract syscall arguments from ISR frame */
+ int syscall_num = isr_frame[17];
+ void *arg1 = (void *) isr_frame[10];
+ void *arg2 = (void *) isr_frame[11];
</file context>
Collaborator
Author
There was a problem hiding this comment.
Fixed! Got the ISR frame offsets wrong - checked boot.c and corrected them.
There was a problem hiding this comment.
Thanks for the feedback! I've saved this as a new learning to improve future reviews.
Adds exception handler for U-mode ecall instructions to dispatch system calls. Extracts syscall number and arguments from the saved register context and returns results through the same mechanism. This design enables privilege separation by allowing user-mode tasks to request kernel services without direct access to privileged operations, addressing the security issue where tasks could bypass isolation by executing in M-mode.
Tasks are now initialized to execute in U-mode rather than M-mode, enabling proper privilege isolation. The MPP field in mstatus is set to USER instead of MACH during context initialization.
HeatCrab
force-pushed
the
u-mode/basic-support
branch
from
9ec8f5c to
e2eec20
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements basic U-mode (User Mode) execution for tasks, addressing Issue #19 where tasks executing in M-mode could bypass isolation by modifying privileged CSRs. This change is essential for PMP (Physical Memory Protection) to function correctly, as PMP only affects U-mode and S-mode.
Tasks now start in user mode and interact with the kernel through a syscall interface based on the ecall trap mechanism. The kernel dispatcher has been refactored to support architecture-specific implementations through weak linking, while the RISC-V backend provides an ecall wrapper following standard calling conventions. Exception handling has been extended to recognize and service user mode traps, maintaining proper privilege boundaries throughout task execution.
Related to #19
Summary by cubic
Enables user-mode execution on RISC-V with an ecall-based syscall path to enforce privilege separation and make PMP effective. Addresses the isolation issue where tasks could modify privileged CSRs in M-mode (Issue #19).
New Features
Refactors
Written for commit e2eec20. Summary will update automatically on new commits.